fix a few -Wstringop-truncation warnings (#565)

* fix -Wstringop-truncation warning in bushnell.

* fix -Wstringop-truncation in alantrl.

* fix -Wstringop-truncation warning in garmin, and a real error.

The real error:
In garmin.cc route_hdr_pr would fail to terminate the
GPS_SWay rte_ident member if latin1 encoded route name was longer
than 255 characters.  Subsequently garmin.cc route_write passes the
GPS_SWay structure to GPS_Command_Send_Route.  GPS_Command_Send_Route
may call GPS_A20[01]_Send which may call GPS_D202_Send.  GPS_D202_Send
assumes that rte_ident is null terminated.

The other change only silences the warning, the character array was
subsequently terminated anyway.

* fix -Wstringop-truncation warning in enigma.

These are nonstrings.  They are not required to be null terminated.

A real reference file caputred from MGL Central 2.0 is added.  In it
you can see that don't care values are not all filled with a particular
value.  We fill them with NULLs are a result of zeroing the structure before
we write it.  You can also see the 1000m offset in altitudes is correct.

* correct reference file mode.

* fix two strncpy warnings that gcc 9.3.0 doesn't issue.

Perhaps it is a gcc bug that these didn't cause warnings.  It seems
to be related to the fact they were the last member in the structure.
In any event, we can copy 1 byte less, we explicitly add a terminator
subsequently.

diff --git a/alan.cc b/alan.cc
index b03143679ffa64184ecd559b2fcb57ecd8567262..1b21c7c57ec985d2dbfae0c1e40080358220ddf7 100644 (file)
--- a/alan.cc
+++ b/alan.cc
@@ -811,24 +811,23 @@ static void trl_track_hdr(const route_head* TL)
     fatal(MYNAME ": Can't store more than %u tracklogs", MAXTRK);
   }
 
-  if (TL->rte_name != nullptr) {
-    strncpy(trkhdr[idx].name, CSTRc(TL->rte_name), TRK_NAME_LEN);
+  if (!TL->rte_name.isEmpty()) {
+    strncpy(trkhdr[idx].name, CSTRc(TL->rte_name), TRK_NAME_LEN - 1);
   }
   if (*(trkhdr[idx].name) == '\0') {
     sprintf(trkhdr[idx].name, "T%03d", idx);
   }
   trkhdr[idx].name[TRK_NAME_LEN-1] = '\0';
 
-  if (TL->rte_desc != nullptr) {
-    strncpy(trkhdr[idx].comment, CSTRc(TL->rte_desc), TRK_COMMENT_LEN);
+  if (!TL->rte_desc.isEmpty()) {
+    strncpy(trkhdr[idx].comment, CSTRc(TL->rte_desc), TRK_COMMENT_LEN - 1);
     int l = strlen(CSTRc(TL->rte_desc));
     if (l < TRK_COMMENT_LEN-1) {
-      memset(trkhdr[idx].comment + l, ' ', TRK_COMMENT_LEN - l);
+      memset(trkhdr[idx].comment + l, ' ', TRK_COMMENT_LEN - 1 - l);
     }
   }
   trkhdr[idx].comment[TRK_COMMENT_LEN-1] = '\0';
 
-  trkhdr[idx].comment[TRK_COMMENT_LEN-1] = '\0';
   trkhdr[idx].occupied = TRK_USED;
   trkhdr[idx].totalpt = 0;
   trkhdr[idx].next = 0;

diff --git a/bushnell.cc b/bushnell.cc
index dd1f6e1bb447203fca3640852c1da54575fe7b32..e7b2a44b0bcce53563936fc4b19c5dd572595d02 100644 (file)
--- a/bushnell.cc
+++ b/bushnell.cc
@@ -233,7 +233,7 @@ bushnell_write_one(const Waypoint* wpt)
   gbfputc(bushnell_get_icon_from_name(wpt->icon_descr), file_out);
   gbfputc(0x01, file_out);  // Proximity alarm.  1 == "off", 3 == armed.
 
-  strncpy(tbuf, CSTRc(wpt->shortname), sizeof(tbuf));
+  strncpy(tbuf, CSTRc(wpt->shortname), sizeof(tbuf) - 1);
   tbuf[sizeof(tbuf)-1] = 0;
   gbfwrite(tbuf, sizeof(tbuf), 1, file_out);
 

diff --git a/enigma.cc b/enigma.cc
index c9c4ca84cf994615df1f29aef9be4e0639915039..850f27b4ef7cd482637e9f754326b4d9b766e4b9 100644 (file)
--- a/enigma.cc
+++ b/enigma.cc
@@ -67,10 +67,10 @@ struct enigma_wpt {
   int32_t                      longitude;
   union wpt_data  data;
   uint8_t         waypoint_type;
-  uint8_t         shortname_len;
-  char            shortname[6];
-  uint8_t         longname_len;
-  char            longname[27];
+  uint8_t         shortname_len;  // number of used characters in shortname
+  char            shortname[6];   // ASCII, unused characters are "don't care" values
+  uint8_t         longname_len;   // number of used characters in longname
+  char            longname[27];   // ASCII, unused characters are "don't care" values
 };
 
 static gbfile* file_in, *file_out;
@@ -181,11 +181,11 @@ enigma_waypt_disp(const Waypoint* wpt)
   }
   if (wpt->shortname != nullptr) {
     ewpt.shortname_len = (uint8_t) min(6, strlen(CSTRc(wpt->shortname)));
-    strncpy(ewpt.shortname, CSTRc(wpt->shortname), 6);
+    memcpy(ewpt.shortname, CSTRc(wpt->shortname), ewpt.shortname_len);
   }
   if (wpt->description != nullptr) {
     ewpt.longname_len = (uint8_t) min(27, strlen(CSTRc(wpt->description)));
-    strncpy(ewpt.longname, CSTRc(wpt->description), 27);
+    memcpy(ewpt.longname, CSTRc(wpt->description), ewpt.longname_len);
   }
   gbfwrite(&ewpt, sizeof(ewpt), 1, file_out);
 }

diff --git a/garmin.cc b/garmin.cc
index 8e0f6b4cafd3c065b103c79bd3b2b54c1f866014..1803d5ce132dcc2f6aeeeb4c4b32c3b5d1f3e04d 100644 (file)
--- a/garmin.cc
+++ b/garmin.cc
@@ -1053,7 +1053,8 @@ route_hdr_pr(const route_head* rte)
   (*cur_tx_routelist_entry)->isrte = 1;
   if (!rte->rte_name.isEmpty()) {
     strncpy((*cur_tx_routelist_entry)->rte_ident, CSTRc(rte->rte_name),
-            sizeof((*cur_tx_routelist_entry)->rte_ident));
+            sizeof((*cur_tx_routelist_entry)->rte_ident) - 1);
+    (*cur_tx_routelist_entry)->rte_ident[sizeof((*cur_tx_routelist_entry)->rte_ident) - 1] = 0;
   }
 }
 
@@ -1103,7 +1104,7 @@ route_waypt_pr(const Waypoint* wpt)
   if (wpt->description.isEmpty()) {
     rte->cmnt[0] = 0;
   } else {
-    strncpy(rte->cmnt, CSTR(wpt->description), sizeof(rte->cmnt));
+    strncpy(rte->cmnt, CSTR(wpt->description), sizeof(rte->cmnt) - 1);
     rte->cmnt[sizeof(rte->cmnt)-1] = 0;
   }
   cur_tx_routelist_entry++;
@@ -1130,7 +1131,7 @@ track_hdr_pr(const route_head* trk_head)
 {
   (*cur_tx_tracklist_entry)->ishdr = true;
   if (!trk_head->rte_name.isEmpty()) {
-    strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident));
+    strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(trk_head->rte_name), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1);
     (*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0;
   } else {
     sprintf((*cur_tx_tracklist_entry)->trk_ident, "TRACK%02d", my_track_count);
@@ -1147,7 +1148,7 @@ track_waypt_pr(const Waypoint* wpt)
   (*cur_tx_tracklist_entry)->alt = (wpt->altitude != unknown_alt) ? wpt->altitude : 1e25;
   (*cur_tx_tracklist_entry)->Time = wpt->GetCreationTime().toTime_t();
   if (!wpt->shortname.isEmpty()) {
-    strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(wpt->shortname), sizeof((*cur_tx_tracklist_entry)->trk_ident));
+    strncpy((*cur_tx_tracklist_entry)->trk_ident, CSTRc(wpt->shortname), sizeof((*cur_tx_tracklist_entry)->trk_ident) - 1);
     (*cur_tx_tracklist_entry)->trk_ident[sizeof((*cur_tx_tracklist_entry)->trk_ident)-1] = 0;
   }
   (*cur_tx_tracklist_entry)->tnew = wpt->wpt_flags.new_trkseg;

diff --git a/reference/mglcentral20.ert b/reference/mglcentral20.ert
new file mode 100644 (file)
index 0000000..53fb328
Binary files /dev/null and b/reference/mglcentral20.ert differ

diff --git a/reference/mglcentral20.gpx b/reference/mglcentral20.gpx
new file mode 100644 (file)
index 0000000..c834714
--- /dev/null
+++ b/reference/mglcentral20.gpx
@@ -0,0 +1,19 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<gpx version="1.0" creator="GPSBabel - https://www.gpsbabel.org" xmlns="http://www.topografix.com/GPX/1/0">
+  <time>1970-01-01T00:00:00Z</time>
+  <bounds minlat="39.908805847" minlon="-105.226089478" maxlat="40.039367676" maxlon="-105.117195129"/>
+  <rte>
+    <rtept lat="40.039367676" lon="-105.226089478">
+      <ele>1611.782</ele>
+      <name>KBDU</name>
+      <cmt>BOULDER MUNI</cmt>
+      <desc>BOULDER MUNI</desc>
+    </rtept>
+    <rtept lat="39.908805847" lon="-105.117195129">
+      <ele>1729.130</ele>
+      <name>KBJC</name>
+      <cmt>ROCKY MOUNTAIN METROPOLITAN</cmt>
+      <desc>ROCKY MOUNTAIN METROPOLITAN</desc>
+    </rtept>
+  </rte>
+</gpx>

diff --git a/testo.d/enigma.test b/testo.d/enigma.test
index 3497127bd03afb1c0a285a0a548b1b97b3586ad3..ce58f04200e733747ac612fb42ac9b956b745085 100644 (file)
--- a/testo.d/enigma.test
+++ b/testo.d/enigma.test
@@ -6,3 +6,10 @@ gpsbabel -i gpx -f ${REFERENCE}/enigma.gpx -o enigma -F ${TMPDIR}/enigma.ert
 compare ${REFERENCE}/enigma-gpsb.ert ${TMPDIR}/enigma.ert
 gpsbabel -i enigma -f ${REFERENCE}/enigma-pfms.ert -o gpx -F ${TMPDIR}/enigma.gpx
 compare ${REFERENCE}/enigma.gpx ${TMPDIR}/enigma.gpx
+
+# mglcentral20.ert was produced by MGL Central version 2.0.
+# don't care values in character string are evident,
+# which makes it impossible to compare "identical" enigma files.
+gpsbabel -i enigma -f ${REFERENCE}/mglcentral20.ert -o gpx -F ${TMPDIR}/mglcentral20.gpx
+compare ${REFERENCE}/mglcentral20.gpx ${TMPDIR}/mglcentral20.gpx
+